Skip to content

Fix thread safety CI Test#251

Open
cavusmustafa wants to merge 35 commits into
ravi9:dev_backend_openvinofrom
cavusmustafa:fix_thread_safety_upstream
Open

Fix thread safety CI Test#251
cavusmustafa wants to merge 35 commits into
ravi9:dev_backend_openvinofrom
cavusmustafa:fix_thread_safety_upstream

Conversation

@cavusmustafa

Copy link
Copy Markdown
Collaborator

Fix: test-thread-safety map::at crash on the OpenVINO backend

For: ravi9/llama.cpp dev_backend_openvino
Patch: fix-thread-safety-get_tensor_ov_name.patch (1 file, ggml-decoder.cpp, +20/−4)
Branch: fix_thread_safety_upstream @ b601163ee (based on c6311c391, no gemma4/MoE code)

Symptom

CI test 33 test-thread-safety fails:

GGML OpenVINO backend std::exception: map::at
graph_compute: ggml_backend_sched_graph_compute_async failed with error -1
llama_decode: failed to decode, ret = -3
One or more threads failed.

(stories15M, -ngl 99 -np 4 -t 2, 3 models × 4 contexts.)

Root cause

get_tensor_ov_name() disambiguates COMPUTE/kvcache tensors that share a ggml name by
appending #<ggml_hash_find(visited_hash_set, tensor)>. That hash value is a hash-set
slot derived from the tensor pointer, so it differs between cgraph instances.

The decoder / infer-request cache (ov_runtime_context) is process-global and keyed
by graph_key (node names + count). The 3 same-architecture models in the test produce
the same graph_key and share one cache entry. On a cache hit:

  • the OV param names captured at compile time embed one instance's hash slots
    (e.g. cache_k_l0#975);
  • they are looked up against the reused decoder's m_inputs, rebuilt by update_io()
    from a different cgraph instance with different hash slots;
  • → the name isn't found → std::map::at throws.

Confirmed via GDB: throw is in GgmlOvDecoder::get_input_ggml_tensor(name="cache_k_l0#975")
m_inputs.at(name). Not concurrency-specific — also reproduces at -np 1 -t 1.

Bisect

commit test-thread-safety
parent of 49852ce (4102b17e1) ✅ "All threads finished without errors"
49852cec6311c391 (HEAD) map::at

49852ce ("remove the unique name in llama.cpp; add new ov name in ov bk") introduced
get_tensor_ov_name, i.e. this regression.

Fix

Derive the suffix from the tensor's topological index in cgraph->nodes[] /
cgraph->leafs[] instead of the pointer-hash slot. That index is identical for
corresponding tensors across graphs of the same structure, so cached OV names resolve on
reuse — while still uniquely disambiguating same-named tensors within a graph.

if ((tensor->flags & GGML_TENSOR_FLAG_COMPUTE) || GgmlOvDecoder::is_kvcache(tensor, nullptr)) {
    for (int i = 0; i < cgraph->n_nodes; i++)
        if (cgraph->nodes[i] == tensor) return std::string(tensor->name) + "#" + std::to_string(i);
    for (int i = 0; i < cgraph->n_leafs; i++)
        if (cgraph->leafs[i] == tensor) return std::string(tensor->name) + "#l" + std::to_string(i);
}
return tensor->name;

Verification (OpenVINO CPU, OV 2026.2.1)

  • test-thread-safety → ✅ "All threads finished without errors" (exit 0).
  • test-backend-ops -b OPENVINO0 → ✅ 2622/2622, 0 fail.
  • Also verified on the gemma4-MoE branch (rebased on the same base): 26B MoE CPU still
    greedy-decodes "France"; no op regressions.

zhaixuejun1993 and others added 30 commits July 1, 2026 13:07
…g_src to recorde the src ggml tensor for OpenVINO dynamic shape infer
…utput_info

Clean node information in OpenVINO backend
…issue

Enable zero-size copy for OpenVINO backend view
enable qwen35

Fix after rebase

remove logging
…t reason: the backend test initializes unary op inputs over a wide range, [-150, 150]. For FP32, exp(x) overflows around x ~= 88.7, so this test can randomly generate values right in or beyond the overflow region
In stateful mode the NEOX RoPE branch fed rank-3 data ([S, n_heads,
head_size]) into the Multiply against the rank-4 cos/sin tables
([1, S, 1, n_dims/2]). That mixed-rank broadcast is miscomputed by the
OpenVINO GPU plugin, corrupting the rotated Q/K and producing garbage
output (e.g. Phi-3-mini). Lift the data to rank-4 before the split/
Multiply so the operands are equal-rank, matching what the TYPE_NORMAL
branch already does. CPU and stateless paths are unaffected.

Phi-3-mini-Q4_K_M, wiki.test perplexity, GPU stateful:
  before: PPL = 27120.43
  after:  PPL = 6.2263   (CPU reference: 6.2251)
…ov name in ov bk; 3) fix issue in arch test & op test with latest code update
Update OpenVINO backend: remove unique name, add new OV name, fix tests
…s-in-llama

OpenVINO Backenb: remove changes in llama.cpp
virajwad and others added 4 commits July 9, 2026 11:23
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Doc change (use x64 Native Tools Command Prompt for VS)
…stances

get_tensor_ov_name() disambiguates COMPUTE/kvcache tensors that share a ggml name
by appending a per-graph suffix. It used ggml_hash_find(visited_hash_set, tensor),
whose value is a hash-set slot derived from the tensor POINTER and therefore differs
between cgraph instances.

The decoder / infer-request cache (ov_runtime_context) is process-global and keyed by
graph_key (node names + count). When several contexts or several same-architecture
models run (e.g. test-thread-safety, which builds 3 models x 4 contexts), they share
one cache under the same graph_key. On a cache hit, the OV param names captured at
compile time (e.g. "cache_k_l0#975") are looked up against the reused decoder's
m_inputs map, which was rebuilt from a *different* cgraph instance with different hash
slots -> std::map::at throws "map::at" and llama_decode fails.

Fix: derive the suffix from the tensor's topological index in cgraph->nodes[] /
cgraph->leafs[] instead of the pointer-hash slot. That index is identical for
corresponding tensors across graphs that share the same structure, so cached OV names
resolve correctly on reuse, while still uniquely disambiguating same-named tensors
within a graph.

Fixes test-thread-safety (test 33) on the OpenVINO backend:
"GGML OpenVINO backend std::exception: map::at" / "One or more threads failed".
Bisected to 49852ce (the commit that introduced get_tensor_ov_name): passes at its
parent, fails from that commit onward. Verified: test-thread-safety now passes, and
test-backend-ops (OPENVINO) stays 2622/2622.
@cavusmustafa
cavusmustafa requested a review from wine99 as a code owner July 10, 2026 19:55
@cavusmustafa
cavusmustafa requested review from wine99 and zhaixuejun1993 and removed request for wine99 July 10, 2026 19:55
@ravi9
ravi9 force-pushed the dev_backend_openvino branch from 7a9b001 to ab6cd7e Compare July 14, 2026 04:06
@ravi9
ravi9 force-pushed the fix_thread_safety_upstream branch from 9badbd5 to b601163 Compare July 14, 2026 04:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants